….1 stub)

Hard lock #3 — dry-run preview before Activate — was previously a no-op. Now,
when a rule is compiled into the draft, the scheduled job replays the most
recent posts through that draft rule (pure evaluation, ZERO actions), and writes
a `${sub}:dryrun:${ruleId}` summary { status, sampledPosts, matched:[{thingId,
authorName, would:[actions]}], note } that the Dashboard renders ("Dry-run
preview (draft rules): r_x: would match N/M recent post(s) → modqueue").

- Samples up to 10 recent posts via reddit.getNewPosts({subredditName, limit}).all()
  → buildPostFactBag → selectMatchingRules([rule], 'onPostSubmit'|'onPostReport', facts).
- A comment-only rule (no post trigger) gets status 'unavailable' with a
  "shadow mode it to see real comments" note (v0.1 has no getNewComments).
- Any Reddit-API/rule failure → status 'unavailable' with the error in the note;
  the handler always returns { status: 'ok' } (never throws — scheduler jobs must).
- Result TTL 7 days. Dashboard reads `${sub}:dryrun:${id}` for each draft rule id.

Tests: routes-scheduler.test.ts gains 6 cases (no-draft, post-rule with matches,
zero matches, comment-only, Reddit-API failure, missing ruleId);
routes-dashboard.test.ts gains a dry-run-display case. test/devvit-testkit.ts
gains a `getNewPosts` double. 157 tests pass; tsc/lint/format clean; acceptance 4/4.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

External code-review pointed out 5 valid issues. All addressed in this commit
on the same branch — PR #44 picks them up automatically.

## Gemini code-assist

### #1 — Manage menu: parallelize per-rule dry-run reads
The previous serial `for` loop multiplied Redis latency by the draft count.
With up to 50 drafts and ~10ms per round-trip the difference is ~500ms vs
~10ms wall time. Switched to `Promise.allSettled([...])` so a single failed
fetch doesn't stop the others.

### #2 — applyManageActions: enforce the 50-rule cap on apply
pause / activate / unpause moves can push a bundle over the 50-rule cap
even when no compose-rule call is made. Added an explicit check before
the dual-write txn — the moderator gets a "Rule cap exceeded (active=N,
draft=N, max=50). Delete a rule first" toast and the bundles stay
untouched. New test in routes-manage.test.ts.

### #3 — Use the configured openaiModel as the fallback (not the literal "manage")
The previous `'manage'` fallback for `bundle.llmModel` broke
`estimateTokenCost()` because 'manage' isn't a key in
`OPENAI_PRICING_USD_PER_TOKEN`. The dashboard shows "(~$0 on manage)" until
a real compile lands. Now we read the configured `openaiModel` setting
through `readOpenaiModel()` (already SELECTION-array-safe per PR #39).

### #4 — Atomic dual-write via WATCH/MULTI/EXEC (the previous PR description's claim was wrong)
The previous `Promise.all([redis.set(activeKey, ...), redis.set(draftKey, ...)])`
was NOT atomic — if the second `set` failed (e.g. plugin RPC blip), pause
could leave a rule absent from active without showing up in draft.
Switched to Devvit's `redis.watch(...).multi().set(...).set(...).exec()`
pattern (same shape executor.ts already uses for audit writes), and check
`exec()`'s null return for the WATCH-aborted case so two moderators
managing rules at the same time get an actionable "Another moderator
changed the rules at the same time — re-open Manage rules" toast.

## CodeRabbit

### #5 — humanizeRule: cap value-stringification at 100 chars
A rule that uses `op: in` against a long allowlist (e.g. 50 banned domains)
would otherwise render thousands of characters into the confirm form's
`compiledSummary` field, blowing past Devvit's modal description budget
and obscuring the actual rule structure. Truncate with `...` after 100
chars in the predicate-tree leaf renderer.

## Test infrastructure
test/devvit-testkit.ts: fakeRedis.watch().exec() now returns `[] | null`
(matching real Devvit Redis MULTI/EXEC semantics) instead of `void`.
Without this, the new WATCH-abort detection in applyManageActions would
fire on every test. Updated `FakeTxn.exec` type accordingly.

## Verification
- `npm run check` 4/4 gates green
- 210 tests pass (1 skipped) — added 1 cap-overflow test in routes-manage.test.ts

5 review issues from Gemini code-assist on PR #45 (refactor/split-server-routes).

## HIGH severity

### #1 Non-ASCII regex made unambiguous
helpers/openai.ts: the `replace(/[<U+0080>-<U+FFFF>]/g, ...)` literal contained
the U+0080 control character invisibly, which made review tools render it
as `/[-<box>]/` and flag it as "matches only `-`". Behaviour was always
correct, but the explicit `/[�-ï¿¿]/g` form removes any doubt.

### #2 WATCH ordering in applyManageActions
routes/manage.ts: the previous shape did GET → modify → WATCH → MULTI →
EXEC, so the optimistic lock didn't actually cover the data we were
about to mutate. Restructured into a CAS retry loop:
  1. WATCH first
  2. GET (under WATCH — uses redis.get because Devvit's `txn.get` queues
     into the transaction and returns a chainable, not the value)
  3. modify in memory
  4. MULTI + SET + EXEC (null exec → another mod's call landed first
     between WATCH and now → loop and retry up to MAX_CAS_ATTEMPTS = 3).
After 3 contended attempts the moderator gets a clear "another mod
changed the rules — re-open and try again" toast.

### #3 Sequential audit-entry hGetAll in undo handler
routes/undo.ts: 100 sequential hGetAll calls could blow the menu
handler's deadline. Switched to `Promise.allSettled([...])` and scan
the resolved array in order so the "most recent first" break-on-found
behaviour is preserved.

## MEDIUM severity

### #4 Sequential audit-entry hGetAll in dashboard
routes/dashboard.ts: same parallelization for the 20-entry recent-actions
fetch and the per-draft-rule dry-run fetch.

### #5 Outdated comment on /internal/trigger/on-app-install
routes/triggers.ts: the comment claimed the submit triggers seed
in-band, but they actually fail-safe by returning ok when no bundle
exists. Updated to describe the real flow: starter rules are seeded
by the deferred /internal/scheduler/seed-on-install task, registered
in devvit.json's scheduler.tasks block.

## Verification
- `npm run check` 4/4 gates green: typecheck + lint + Prettier + 211 tests
  (1 skipped) + 3 @devvit/test cases + acceptance G1-G4
- All PR #45 tests still pass with no test-side modifications

5 review issues from Gemini code-assist + CodeRabbit on PR #49.

## Major

### CodeRabbit #4 — pending consumption now atomic (WATCH/MULTI/DEL/EXEC)
Two concurrent submits with the same `pendingId` (back-button + re-submit,
double-click on Save, multi-tab moderator) could both read the entry
before either deleted it, doubling the bundle write + dry-run schedule.
Replaced the plain GET-then-DEL with a WATCH/MULTI/DEL/EXEC round-trip.
Whichever caller commits first wins; the loser sees `exec() == null` and
gets an actionable "Another submission consumed this confirmation" toast.

Updated FakeTxn to match (added `del()` method on the txn object — the
production Devvit Redis client supports it, the fake didn't).

### CodeRabbit #5 — daily compile counter now bumps at compile time
Quota was previously incremented inside `persistRuleAndStartDryRun()`,
which only runs on Save. A moderator could repeatedly hit Compile +
Cancel and burn through OpenAI tokens without the per-day counter ever
moving. Moved the increment to immediately after a successful OpenAI
return in compose-rule-submit. The token cost is real either way, so
the quota now reflects it either way.

## Medium

### Gemini #1 — dashboard cancelLabel no longer says "Don't show intro again"
Devvit's Cancel button does NOT trigger the form submit handler, so
clicking the misnamed button could not actually persist
`dismissOnboarding` — it just closed the modal. Reverted the label to
"Cancel"; the dismissOnboarding boolean toggle + Close (acceptLabel) is
the real opt-out path that submits the form values.

## Minor

### CodeRabbit #2 — verify script tolerates non-int SLOW_MO
`int(os.environ.get("SLOW_MO", "0"))` crashed if a user passed e.g.
`SLOW_MO=fast`. Added `_safe_int` helper that warns + falls back to the
default rather than ending the recording session before it starts.

### CodeRabbit #3 — atomic SET + TTL via the `expiration` option
Previous shape was `redis.set(key, value)` then `redis.expire(key, ttl)`,
which could leak a TTL-less pending key if expire failed after set
succeeded. Switched to `redis.set(key, value, { expiration: new Date(...) })`
which is the same single round-trip the rollback writer in executor.ts
already uses.

## Verification
- `npm run check` 4/4 gates green (211 unit/integration tests)
- All compose tests still pass with the new pending-id flow
- Race condition smoke: a second compose-confirm-submit call against the
  same pendingId now returns the "another submission consumed" toast
  instead of double-persisting (previously: double bundle write).

…ctions/setup-node-6

ci: Bump actions/setup-node from 4 to 6

….1 stub)

Hard lock #3 — dry-run preview before Activate — was previously a no-op. Now,
when a rule is compiled into the draft, the scheduled job replays the most
recent posts through that draft rule (pure evaluation, ZERO actions), and writes
a `${sub}:dryrun:${ruleId}` summary { status, sampledPosts, matched:[{thingId,
authorName, would:[actions]}], note } that the Dashboard renders ("Dry-run
preview (draft rules): r_x: would match N/M recent post(s) → modqueue").

- Samples up to 10 recent posts via reddit.getNewPosts({subredditName, limit}).all()
  → buildPostFactBag → selectMatchingRules([rule], 'onPostSubmit'|'onPostReport', facts).
- A comment-only rule (no post trigger) gets status 'unavailable' with a
  "shadow mode it to see real comments" note (v0.1 has no getNewComments).
- Any Reddit-API/rule failure → status 'unavailable' with the error in the note;
  the handler always returns { status: 'ok' } (never throws — scheduler jobs must).
- Result TTL 7 days. Dashboard reads `${sub}:dryrun:${id}` for each draft rule id.

Tests: routes-scheduler.test.ts gains 6 cases (no-draft, post-rule with matches,
zero matches, comment-only, Reddit-API failure, missing ruleId);
routes-dashboard.test.ts gains a dry-run-display case. test/devvit-testkit.ts
gains a `getNewPosts` double. 157 tests pass; tsc/lint/format clean; acceptance 4/4.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…ng (Phase 1.7b Tier 1+2+3)

Best-practice UX rework for the 5 deferred audit findings (#2 #3 #5 #7 #10) plus
4 new items (#A empty states, #B delete confirm, #C onboarding, #D token cost).
Design + tier rationale: claudedocs/2026-05-14-ux-best-practices-plan.md.

## Tier 1 — compose flow (audit #2 #5 #7)

### #2 Compile preview confirmation form (composeConfirmForm)
Compile no longer persists straight to draft. After validation, the moderator
sees a confirm form with the rule rendered in plain English (humanizeRule()),
the original sentence, and the per-compile token cost (~$0.0006 on
gpt-5.4-mini). Tick "Edit instead of save" to re-open compose with the
original NL pre-filled. Save persists draft + schedules dry-run via the new
persistRuleAndStartDryRun() helper. Defence-in-depth: serialized rule is
re-validated before write.

New endpoint: /internal/form/compose-confirm-submit
New form: composeConfirmForm

### #5 Clarification turn limit (3 rounds)
Server-side counter shipped as a disabled paragraph field. After round 3, an
oscillating LLM gets refused with an actionable toast ("Try rephrasing more
concretely"), instead of opening yet another modal. Each clarify modal
description prefixed with "(Round X of 3)" so the moderator sees how many
tries remain.

### #7 Editable original rule in clarify modal
Removed `disabled: true` on the Original rule field. helpText now says
"Re-compile uses this text plus your answer below."

## Tier 2 — Manage rules menu + dashboard hardening (audit #3 #10 #A #B #D)

### #3 + #10 Per-rule control surface (Manage rules)
New "vibe-mod: Manage rules" menu (subreddit-level, mod-only). Renders one
form-group per rule with a `select` of available actions:
- Drafts: Keep / Activate (shadow 24h) / Activate immediately / Delete
- Active shadow rules: Keep / Promote shadow → live / Pause / Delete
- Active live rules: Keep / Pause / Delete
Submit applies all non-destructive actions atomically (one redis.set per
bundle) via the new applyManageActions() helper. Dashboard becomes
read-only — its old "Activate N drafts" boolean removed.

New endpoints: /internal/menu/manage-rules,
               /internal/form/manage-rules-submit,
               /internal/form/manage-delete-confirm
New forms: manageRulesForm, manageDeleteConfirmForm

### #B Delete confirmation
Any `delete` action in the Manage submit forwards to a confirm form listing
the rules about to disappear, with an explicit "I understand this is
permanent" boolean. The pending action map round-trips through a disabled
paragraph carrier so the second-step submit knows what to apply.

### #A Empty states
- Manage menu emits a guided toast when there are zero rules at all.
- Dashboard description shows "No rules yet — open ⋯ → Compose" when both
  rules count and recent actions are zero.

### #D Token cost transparency
- Dashboard description: "Tokens used (lifetime): N in / N out (~$X on
  gpt-5.4-mini)" via the new estimateTokenCost() helper.
- Compose confirm form shows per-compile cost so the moderator sees the
  unit economics before saving.

## Tier 3 — Onboarding (audit #C)

Dashboard shows a 3-step welcome card on first visit (per-sub Redis flag
`${sub}:onboarding:dismissed`). Cancel-label switches to "Don't show intro
again" and ticks dismissOnboarding to persist the flag. New key in
src/shared/redis-keys.ts.

## Verification
- `npm run check` 4/4 gates green
- 209 unit + integration tests (1 skipped):
  - 33 compose tests (8 updated for new 2-step flow + 4 new for #2/#5/#7)
  - 13 dashboard tests (4 updated for read-only + 4 new for onboarding/empty)
  - 17 manage tests (NEW file routes-manage.test.ts: menu render, all 5 actions, delete confirm round-trip)
  - existing trigger / scheduler / settings / undo / evaluator / executor unchanged

## Why a single PR
Tier 1+2+3 all touch overlapping files (index.ts, devvit.json, the dashboard form
shape). Splitting would force back-and-forth conflict resolution. Logical
commits inside this PR (split when applicable in follow-up).

Refs: claudedocs/2026-05-14-ux-best-practices-plan.md (Phase 1.7a),
      claudedocs/2026-05-14-compose-flow-audit.md (audit baseline),
      docs/demo-scenario.md (downstream dependency)

External code-review pointed out 5 valid issues. All addressed in this commit
on the same branch — PR #44 picks them up automatically.

## Gemini code-assist

### #1 — Manage menu: parallelize per-rule dry-run reads
The previous serial `for` loop multiplied Redis latency by the draft count.
With up to 50 drafts and ~10ms per round-trip the difference is ~500ms vs
~10ms wall time. Switched to `Promise.allSettled([...])` so a single failed
fetch doesn't stop the others.

### #2 — applyManageActions: enforce the 50-rule cap on apply
pause / activate / unpause moves can push a bundle over the 50-rule cap
even when no compose-rule call is made. Added an explicit check before
the dual-write txn — the moderator gets a "Rule cap exceeded (active=N,
draft=N, max=50). Delete a rule first" toast and the bundles stay
untouched. New test in routes-manage.test.ts.

### #3 — Use the configured openaiModel as the fallback (not the literal "manage")
The previous `'manage'` fallback for `bundle.llmModel` broke
`estimateTokenCost()` because 'manage' isn't a key in
`OPENAI_PRICING_USD_PER_TOKEN`. The dashboard shows "(~$0 on manage)" until
a real compile lands. Now we read the configured `openaiModel` setting
through `readOpenaiModel()` (already SELECTION-array-safe per PR #39).

### #4 — Atomic dual-write via WATCH/MULTI/EXEC (the previous PR description's claim was wrong)
The previous `Promise.all([redis.set(activeKey, ...), redis.set(draftKey, ...)])`
was NOT atomic — if the second `set` failed (e.g. plugin RPC blip), pause
could leave a rule absent from active without showing up in draft.
Switched to Devvit's `redis.watch(...).multi().set(...).set(...).exec()`
pattern (same shape executor.ts already uses for audit writes), and check
`exec()`'s null return for the WATCH-aborted case so two moderators
managing rules at the same time get an actionable "Another moderator
changed the rules at the same time — re-open Manage rules" toast.

## CodeRabbit

### #5 — humanizeRule: cap value-stringification at 100 chars
A rule that uses `op: in` against a long allowlist (e.g. 50 banned domains)
would otherwise render thousands of characters into the confirm form's
`compiledSummary` field, blowing past Devvit's modal description budget
and obscuring the actual rule structure. Truncate with `...` after 100
chars in the predicate-tree leaf renderer.

## Test infrastructure
test/devvit-testkit.ts: fakeRedis.watch().exec() now returns `[] | null`
(matching real Devvit Redis MULTI/EXEC semantics) instead of `void`.
Without this, the new WATCH-abort detection in applyManageActions would
fire on every test. Updated `FakeTxn.exec` type accordingly.

## Verification
- `npm run check` 4/4 gates green
- 210 tests pass (1 skipped) — added 1 cap-overflow test in routes-manage.test.ts

5 review issues from Gemini code-assist on PR #45 (refactor/split-server-routes).

## HIGH severity

### #1 Non-ASCII regex made unambiguous
helpers/openai.ts: the `replace(/[<U+0080>-<U+FFFF>]/g, ...)` literal contained
the U+0080 control character invisibly, which made review tools render it
as `/[-<box>]/` and flag it as "matches only `-`". Behaviour was always
correct, but the explicit `/[�-ï¿¿]/g` form removes any doubt.

### #2 WATCH ordering in applyManageActions
routes/manage.ts: the previous shape did GET → modify → WATCH → MULTI →
EXEC, so the optimistic lock didn't actually cover the data we were
about to mutate. Restructured into a CAS retry loop:
  1. WATCH first
  2. GET (under WATCH — uses redis.get because Devvit's `txn.get` queues
     into the transaction and returns a chainable, not the value)
  3. modify in memory
  4. MULTI + SET + EXEC (null exec → another mod's call landed first
     between WATCH and now → loop and retry up to MAX_CAS_ATTEMPTS = 3).
After 3 contended attempts the moderator gets a clear "another mod
changed the rules — re-open and try again" toast.

### #3 Sequential audit-entry hGetAll in undo handler
routes/undo.ts: 100 sequential hGetAll calls could blow the menu
handler's deadline. Switched to `Promise.allSettled([...])` and scan
the resolved array in order so the "most recent first" break-on-found
behaviour is preserved.

## MEDIUM severity

### #4 Sequential audit-entry hGetAll in dashboard
routes/dashboard.ts: same parallelization for the 20-entry recent-actions
fetch and the per-draft-rule dry-run fetch.

### #5 Outdated comment on /internal/trigger/on-app-install
routes/triggers.ts: the comment claimed the submit triggers seed
in-band, but they actually fail-safe by returning ok when no bundle
exists. Updated to describe the real flow: starter rules are seeded
by the deferred /internal/scheduler/seed-on-install task, registered
in devvit.json's scheduler.tasks block.

## Verification
- `npm run check` 4/4 gates green: typecheck + lint + Prettier + 211 tests
  (1 skipped) + 3 @devvit/test cases + acceptance G1-G4
- All PR #45 tests still pass with no test-side modifications

5 review issues from Gemini code-assist + CodeRabbit on PR #49.

## Major

### CodeRabbit #4 — pending consumption now atomic (WATCH/MULTI/DEL/EXEC)
Two concurrent submits with the same `pendingId` (back-button + re-submit,
double-click on Save, multi-tab moderator) could both read the entry
before either deleted it, doubling the bundle write + dry-run schedule.
Replaced the plain GET-then-DEL with a WATCH/MULTI/DEL/EXEC round-trip.
Whichever caller commits first wins; the loser sees `exec() == null` and
gets an actionable "Another submission consumed this confirmation" toast.

Updated FakeTxn to match (added `del()` method on the txn object — the
production Devvit Redis client supports it, the fake didn't).

### CodeRabbit #5 — daily compile counter now bumps at compile time
Quota was previously incremented inside `persistRuleAndStartDryRun()`,
which only runs on Save. A moderator could repeatedly hit Compile +
Cancel and burn through OpenAI tokens without the per-day counter ever
moving. Moved the increment to immediately after a successful OpenAI
return in compose-rule-submit. The token cost is real either way, so
the quota now reflects it either way.

## Medium

### Gemini #1 — dashboard cancelLabel no longer says "Don't show intro again"
Devvit's Cancel button does NOT trigger the form submit handler, so
clicking the misnamed button could not actually persist
`dismissOnboarding` — it just closed the modal. Reverted the label to
"Cancel"; the dismissOnboarding boolean toggle + Close (acceptLabel) is
the real opt-out path that submits the form values.

## Minor

### CodeRabbit #2 — verify script tolerates non-int SLOW_MO
`int(os.environ.get("SLOW_MO", "0"))` crashed if a user passed e.g.
`SLOW_MO=fast`. Added `_safe_int` helper that warns + falls back to the
default rather than ending the recording session before it starts.

### CodeRabbit #3 — atomic SET + TTL via the `expiration` option
Previous shape was `redis.set(key, value)` then `redis.expire(key, ttl)`,
which could leak a TTL-less pending key if expire failed after set
succeeded. Switched to `redis.set(key, value, { expiration: new Date(...) })`
which is the same single round-trip the rollback writer in executor.ts
already uses.

## Verification
- `npm run check` 4/4 gates green (211 unit/integration tests)
- All compose tests still pass with the new pending-id flow
- Race condition smoke: a second compose-confirm-submit call against the
  same pendingId now returns the "another submission consumed" toast
  instead of double-persisting (previously: double bundle write).